tools: libxl: Take the userdata lock around maxmem changes
authorIan Campbell <ian.campbell@citrix.com>
Tue, 23 Jun 2015 14:58:32 +0000 (15:58 +0100)
committerIan Campbell <ian.campbell@citrix.com>
Fri, 26 Jun 2015 11:59:10 +0000 (12:59 +0100)
commit5f33fa2bca6354fad1decfeda723c046311e85cc
tree3f526ad3ce6fa453bcb84993d08fec93c94afd65
parent46bc4edd8c85f6a230939be0600f13e29d9a0d0e
tools: libxl: Take the userdata lock around maxmem changes

There is an issue in libxl_set_memory_target whereby the target and
the max mem can get out of sync, this is because the call the
xc_domain_setmaxmem is not tied in any way to the xenstore transaction
which controls updates to the xenstore side of things.

Consider a domain with 1M of RAM (==target and maxmem for the sake of
argument) and two simultaneous calls to libxl_set_memory_target, both
with relative=0 and enforce=1, one with target=3 and the other with
target=5.

target=5 call                   target=3 call

transaction start
                                transaction start
write target=5 to xenstore
                                write target=3 to xenstore
setmaxmem(5)
                                setmaxmem(3)

transaction commit = success
                                transaction commit = EAGAIN

At this point maxmem=3 while target=5.

In reality the target=3 case will the retry and eventually (hopefully)
succeed with target=maxmem=3, however the bad state will persist for
some window which is undesirable. On failure other than EAGAIN all
bets are off anyway, but in that case we will likely stick in the bad
state until someone else sets the memory).

To fix this we slightly abuse the userdata lock which is used to
protect updates to the domain's json configuration. Abused because
maxmem is not actually stored in there, but is kept by Xen. However
the lock protects some semantically similar things and is convenient
to use here too.

libxl_domain_setmaxmem also takes the lock, since it reads
memory/target from xenstore before calling xc_domain_setmaxmem there
is a small (but perhaps not very interesting) race there too.

There is on more use of xc_domain_setmaxmem in libxl__build_pre.
However taking a lock around this would be tricky since the xenstore
parts are not done until libxl__build_post. I think this one could be
argued to be OK since the domid is not "public" yet, that is it has
not been returned to the application yet (as the result of the create
operation). Toolstacks which go round fiddling with random domid's
which they find lying on the floor should be taught to do better.

Add a doc note that taking the userdata lock requires the CTX_LOCK to
be held.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
tools/libxl/libxl.c
tools/libxl/libxl_internal.h